Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coq_kernel: init at 1.6.0 #171202

Merged
merged 1 commit into from
Nov 21, 2023
Merged

coq_kernel: init at 1.6.0 #171202

merged 1 commit into from
Nov 21, 2023

Conversation

thomasjm
Copy link
Contributor

@thomasjm thomasjm commented May 2, 2022

Description of changes

This is a Jupyter kernel for Coq, the formal proof management system.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 2, 2022
@thomasjm
Copy link
Contributor Author

@SuperSandro2000 any chance I could trouble you to review this? I think it should be straightforward, I did it the same way as other Jupyter kernels I've packaged in the past.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@siraben don't you know coq?

pkgs/applications/editors/jupyter-kernels/coq/kernel.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/jupyter-kernels/coq/kernel.nix Outdated Show resolved Hide resolved
pkgs/applications/editors/jupyter-kernels/coq/kernel.nix Outdated Show resolved Hide resolved
@siraben
Copy link
Member

siraben commented Jun 24, 2022

@siraben don't you know coq?

I do, but I'm not very familiar with Jupyter. I'll review this when I have some time.

@thomasjm
Copy link
Contributor Author

Any chance we can get this merged soon? Coq familiarity isn't really required, you just have to start up the Jupyter notebook and confirm that it successfully runs Coq code. It works for me when doing the following:

nix run nixpkgs#nixpkgs-review pr 171202
$(nix-build -E 'with import ./. {}; jupyter.override { definitions = { coq = coq-kernel.definition; }; }')/bin/jupyter-notebook

@thomasjm
Copy link
Contributor Author

@SuperSandro2000 @siraben ping again on this.

@skeuchel
Copy link
Contributor

skeuchel commented Dec 7, 2022

I confirm that I can start the notebook and run Coq code. How would one go about adding coqPackages?

Currently a dependency is broken for which I opened a PR #205043.

@thomasjm
Copy link
Contributor Author

thomasjm commented Dec 8, 2022

I was actually looking at the metakernel breakage yesterday and realized coq_jupyter doesn't actually require it. Just removed it and rebased on master.

@thomasjm
Copy link
Contributor Author

thomasjm commented Dec 8, 2022

How would one go about adding coqPackages?

I'm not sure, the manual seems to only describe how to add new ones. If you figure it out please lmk!

@thomasjm
Copy link
Contributor Author

thomasjm commented Dec 8, 2022

Actually, it seems like coqPackages are meant to be installed alongside coq in the same environment. For example, if I build coqPackages.ceres, it produces a lib path result/lib/coq/8.16/user-contrib/Ceres. Presumably coq will search these kinds of paths when you do an Import.

However, it's not working too well for me. I tried installing coq alongside coqPackages.ceres, and the import still failed. I notice coq alone produces lib/coq/user-contrib so perhaps there's a slight mismatch here.

EDIT: it does work in coqide though. That must be setting up search paths differently.

@thomasjm
Copy link
Contributor Author

thomasjm commented Dec 8, 2022

Aha I figured it out, based on the bullet point about extraInstallFlags here: https://nixos.org/manual/nixpkgs/stable/#coq-packages-attribute-sets-coqpackages.

Just pushed a commit adding a definitionWithPackages function.

@skeuchel
Copy link
Contributor

skeuchel commented Dec 8, 2022

Just pushed a commit adding a definitionWithPackages function.

That should be pretty useful. Pinning versions is common in this space, so could you also make the coq version configurable? I would like to pass say coq_8_15 along with some coqPackages_8_15. I will test it again after that, and then it should be ready before more feature creep sets in ;)

@thomasjm
Copy link
Contributor Author

thomasjm commented Dec 8, 2022

I think it's already configurable via override, which AFAIK is the standard pattern.

@thomasjm
Copy link
Contributor Author

thomasjm commented Dec 8, 2022

What I'd really like to do is open a PR to either attach the coqPackages for a given version to the coq, or even better, add a coq.withPackages function.

@skeuchel
Copy link
Contributor

skeuchel commented Dec 8, 2022

I've tested it with adding some coqPackages and also overriding the coq version. Everything works so it LGTM.

@thomasjm
Copy link
Contributor Author

thomasjm commented Dec 9, 2022

Great! Let's see if we can get it merged now.

@thomasjm
Copy link
Contributor Author

thomasjm commented May 3, 2023

@SuperSandro2000 ? This is still ready to go

@thomasjm thomasjm requested a review from jonringer August 6, 2023 00:56
@thomasjm
Copy link
Contributor Author

thomasjm commented Aug 6, 2023

Hi @jonringer, I'm hoping to find a reviewer for this and a few other Jupyter-related PRs. I saw you committed the initial IRuby PR so I was wondering if you could take a look here?

@thomasjm
Copy link
Contributor Author

thomasjm commented Sep 8, 2023

I've just updated this to be consistent with the latest Jupyter stuff. I think it's ready to merge!

@teto teto added the 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab label Nov 3, 2023
@GTrunSec
Copy link
Contributor

Could you rebase your changes with the latest nixpkgs?

@GTrunSec GTrunSec closed this Nov 16, 2023
@GTrunSec GTrunSec reopened this Nov 16, 2023
@thomasjm
Copy link
Contributor Author

Done @GTrunSec

@GTrunSec
Copy link
Contributor

Test: nix run --impure --expr 'with import ./. {}; jupyter-console.withSingleKernel (coq-kernel.definitionWithPackages [coqPackages.bignums])'

nix run --impure --expr 'with import ./. {}; jupyter-console.withSingleKernel (coq-kernel.definitionWithPackages [coqPackages.bignums])'
Jupyter console 6.6.3

The Coq Proof Assistant, version 8.18.0
compiled with OCaml 4.14.1

In [1]: Compute (1 + 1).
Out[2]:
     = 2
     : nat

Thanks, LGTM

@thomasjm
Copy link
Contributor Author

Cool! I guess now we ping @teto that this kernel is ready to go?

@GTrunSec
Copy link
Contributor

we should ping @natsukium jupyter team.

@thomasjm thomasjm requested a review from teto November 21, 2023 23:49
@teto teto merged commit 1383b84 into NixOS:master Nov 21, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants